feat(cli): bundle pg-delta for database workflows - #6102
Conversation
avallete
added
the
run-live-e2e-ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8078b53b04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Reads the rollout flag once when the command-scoped layer is constructed. */ | ||
| export const legacyPgDeltaEngineLayer = Layer.unwrap( | ||
| Effect.gen(function* () { | ||
| const raw = process.env[FLAG]; |
There was a problem hiding this comment.
When SUPABASE_USE_PG_DELTA_NEXT=false is defined only in supabase/.env, this layer is constructed before the handler loads project environment values and therefore always selects the default next engine. This also disagrees with legacyDbPushCore, which resolves the same flag through toml.envLookup, so one project can warm the legacy catalog during db push but still execute the next engine for db diff, db pull, generate, and sync. Resolve the selector from the project-aware environment so the documented opt-out behaves consistently.
Useful? React with 👍 / 👎.
| const rendered = libraries.renderPlanFiles(generatedPlan, { | ||
| allowDrops: input.allowDrops, | ||
| }); |
There was a problem hiding this comment.
When a project configures [experimental.pgdelta].format_options, the next-engine diff and declarative-plan paths silently discard it: the engine inputs carry formatOptions, but the adapter inputs do not, and both renderer calls receive only allowDrops. Consequently db diff, migration-style db pull, and declarative sync ignore overrides such as lowercase keywords, indentation, or maximum width despite the command documentation promising that behavior; thread the parsed options through these adapter operations and into renderPlanFiles.
AGENTS.md reference: apps/cli/AGENTS.md:L483-L485
Useful? React with 👍 / 👎.
| * Management API. The synchronous `docker info` probe is read-only and runs once | ||
| * when this helper module is collected. | ||
| */ | ||
| export const describeDockerLive = describe.skipIf(!hasDockerDaemon()); |
There was a problem hiding this comment.
On any developer or CI host where Docker happens to be available, invoking the test:live target without live credentials now collects and runs this suite instead of leaving it inert, causing it to start a real local Supabase stack and run a scenario with a 15-minute timeout. The repository explicitly uses the configured live environment as the signal that these expensive suites may run, even for Docker-only commands, so this helper should retain that gate rather than probing Docker alone.
AGENTS.md reference: apps/cli/AGENTS.md:L438-L443
Useful? React with 👍 / 👎.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@0ebe58cd26926dc5333bbb7958240c024c48fb33Preview package for commit |
|
Provisioned validation against exact head
The labeled dispatch check itself cannot reach the internal repository because its GitHub App is not installed there, so this run was dispatched manually against the same SHA. |
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| "@parcel/watcher": "^2.6.0", | ||
| "@supabase/api": "workspace:*", | ||
| "@supabase/config": "workspace:*", | ||
| "@supabase/pg-delta": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-delta@ad62ae432865f67bb359a8183a2b3279fa9ebccb", |
There was a problem hiding this comment.
🟠 Severity: HIGH
Both @supabase/pg-delta (v1.0.0-alpha.33) and @supabase/pg-topo (v1.0.0-alpha.5) are sourced from pkg.pr.new, a third-party preview registry (StackBlitz), rather than the official npm registry. These pre-release packages are compiled into the CLI binary via bun build --compile and distributed to end users. They run in-process with direct access to user Postgres connection pools and can extract full database schemas. Shipping a production CLI binary with code from an ephemeral, unofficial PR-preview channel that has not been formally published to npm represents a supply-chain risk.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Replace the pkg.pr.new ephemeral preview-registry URLs for @supabase/pg-delta and @supabase/pg-topo with official npm registry version specifiers once the packages are formally published to npm. As acknowledged in the PR description, these are temporarily pinned to a PR-preview commit (pg-toolbelt PR #299 @ 951daa9). The resolution steps are: (1) Ensure the @supabase/pg-delta and @supabase/pg-topo packages are published to the official npm registry under the @supabase scope. (2) Replace lines 58–59 with proper semver specifiers, e.g. "@supabase/pg-delta": "^1.0.0-alpha.33" and "@supabase/pg-topo": "^1.0.0-alpha.5". (3) Do not ship a production CLI binary that bundles packages sourced from pkg.pr.new or any other ephemeral, unofficial preview channel, as those artifacts are not subject to the same security controls as the official npm registry.
There was a problem hiding this comment.
💡 Codex Review
Detect collisions by migration version, not full path
When a multi-segment pg-delta plan is generated in the same second as an existing migration with a different name—or overlaps future-dated segments from a prior run—checking only w.path misses the collision because the filenames differ. This writes multiple files with the same 14-digit version; local migration loading accepts both, but schema_migrations.version is a primary key, so a pull can fail while repairing history and later push/reset operations can fail while applying the duplicate version. Check every candidate version against all existing migration filenames rather than only the generated pathname.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const shadow = yield* shadowService.provision({ | ||
| schema: input.schema, | ||
| ...(input.projectRef !== undefined ? { projectRef: input.projectRef } : {}), | ||
| }); |
There was a problem hiding this comment.
With the supported [db] major_version = 15 and the default next implementation, every pg-delta db diff and migration-style db pull fails before comparing databases: this call provisions both shadows, the second shadow's setup rejects every major other than 17 in SetupPgDeltaNextDeclarativeShadowDatabase, yet this operation only reads shadow.migrationsUrl and never uses declarativeUrl. Provision only the migrated shadow for diffDatabase (and explicit migrations endpoints), reserving the two-shadow path for declarative planning, so ordinary database diffs continue to work for supported Postgres 15 projects.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
| migrationsPort, err := allocatePgDeltaNextPort(dependencies.freePort, 0) | ||
| if err != nil { | ||
| return PgDeltaNextShadow{}, err | ||
| } | ||
| migrationsContainer, err := dependencies.create(ctx, migrationsPort) |
There was a problem hiding this comment.
When the CLI runs in a dev container or uses a remote TCP DOCKER_HOST, dependencies.freePort calls GetFreeHostPort, which probes 127.0.0.1 in the CLI process's network namespace, while CreateShadowDatabase publishes the returned port on the Docker daemon's host. A port that is free locally can already be occupied on that host, causing either shadow container creation to fail nondeterministically even though utils.Config.Hostname otherwise supports connecting to that external host. Let Docker allocate each host port and inspect the resulting binding, or otherwise reserve ports in the daemon host's namespace.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
avallete
removed
the
run-live-e2e-ci
| "@supabase/api": "workspace:*", | ||
| "@supabase/config": "workspace:*", | ||
| "@supabase/pg-delta": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-delta@ad62ae432865f67bb359a8183a2b3279fa9ebccb", | ||
| "@supabase/pg-topo": "https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-topo@ad62ae432865f67bb359a8183a2b3279fa9ebccb", |
There was a problem hiding this comment.
🟠 Severity: HIGH
@supabase/pg-topo is sourced from pkg.pr.new (StackBlitz ephemeral PR-preview registry), not the official npm registry. It runs in-process with direct Postgres pool access and is compiled into the distributed CLI binary, presenting the same unresolved supply-chain risk as @supabase/pg-delta.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Replace the ephemeral pkg.pr.new preview URL for @supabase/pg-topo (and the companion @supabase/pg-delta on line 58) with official npm registry version specifiers once the packages are published. The PR body acknowledges these pins are temporary (pending pg-toolbelt PR #299 publication). Until then, this PR should not be merged into a release branch, as the ephemeral preview tarball is sourced from an untrusted, non-audited registry and is compiled directly into the distributed CLI binary with in-process Postgres pool access. When @supabase/pg-topo is published to npm, replace line 59 with: "@supabase/pg-topo": "^<published-version>". Apply the same change to @supabase/pg-delta on line 58.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4861957ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| paths.sort((left, right) => left.name.localeCompare(right.name)); |
There was a problem hiding this comment.
When declarative filenames differ by case or non-ASCII characters, localeCompare applies locale-sensitive collation—for example, a.sql can sort before Z.sql—rather than the Go implementation's bytewise sort.Strings ordering. Because this ordered array is passed to planSchemaFiles, projects whose DDL files rely on lexical ordering can be loaded differently under the default next engine and fail or produce a different desired schema; use a locale-independent code-point comparison to preserve legacy behavior.
AGENTS.md reference: apps/cli/AGENTS.md:L483-L485
Useful? React with 👍 / 👎.
| yield* rejectBlockingDiagnostic("declarativePlan", result.diagnostics); | ||
| return { | ||
| ...normalizeNextDiff(result, debugDirectory), |
There was a problem hiding this comment.
When planSchemaFiles cannot load a declarative statement and reports it through result.skipped—for example, an out-of-scope CREATE ROLE—this return path discards the skipped list and continues with the partial plan. If all requested statements are skipped, db schema declarative sync can even print No schema changes found, falsely implying that the migrations state matches the files; fail the operation or surface each skipped statement before accepting the plan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Check migration version collisions across all filenames
When a multi-unit plan assigns a version already used by another migration with a different name, this exact-path check reports no collision and writes a second <same-version>_*.sql file. This can occur when a migration was created in the same second as the pull/diff, or when a later plan unit's future-dated timestamp overlaps an existing file; db pull then upserts that version in schema_migrations, replacing the existing history row while both local files remain. Check for any ${version}_*.sql entry before accepting the generated set, rather than only each proposed pathname.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (parsed !== undefined) return parsed; | ||
| throw new LegacyPgDeltaEngineError({ | ||
| message: "failed to parse Postgres connection string for pg-delta", | ||
| cause: endpoint.ref.replace(/:[^:@/]+@/, ":***@"), |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
The password redaction regex :[^:@/]+@ stops at the first @ character in the password. For passwords containing @ (e.g., user:p@ssword@host), only p is redacted, leaving the remainder (ssword) exposed in the error cause that propagates through logs/error reporting.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Update the password redaction regex to handle passwords containing @ characters by adding an optional repeating group (?:@[^@/]+)* that captures additional @-delimited segments within the password before the final credential-separator @. Change the regex from /:[^:@/]+@/ to /:[^:@/]+(?:@[^@/]+)*@/. This causes the engine to backtrack correctly and match the full password (including embedded @ characters) up to the last @ before the host, rather than stopping at the first @ inside the password.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| cause: endpoint.ref.replace(/:[^:@/]+@/, ":***@"), | |
| cause: endpoint.ref.replace(/:[^:@/]+(?:@[^@/]+)*@/, ":***@"), |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ebe58cd26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const parsed: unknown = JSON.parse(raw); | ||
| if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return undefined; |
There was a problem hiding this comment.
When [experimental.pgdelta].format_options = "null", this branch converts the explicit opt-out to undefined, so legacyPgDeltaNextExportOptions omits format exactly as it does when the setting is absent. The documented contract requires those states to differ—unset uses default formatting while null emits raw SQL—so db pull --declarative and declarative generate cannot honor the opt-out under the default next engine. Preserve null as a distinct value through buildSchemaExport.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
| pending.push(full); | ||
| continue; | ||
| } | ||
| if (path.extname(entry).toLowerCase() !== ".sql") continue; |
There was a problem hiding this comment.
When a declarative directory contains a file such as tables.SQL, lowercasing the extension causes the next engine to load and apply it, whereas the Go reference's hashDeclarativeSchemas and Glob.SQLFiles accept only an exact .sql extension. A project can therefore generate migrations from statements the stable Go workflow ignored; compare the extension without lowercasing to preserve the legacy command contract.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
| | Old command | TS status | TS command path or `missing` | Missing flags/params | Extra TS flags/params | Notes | | ||
| | --------------------------------- | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -------------------- | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `db diff` | `ported` | `legacy/commands/db/diff/` | `n/a` | `n/a` | Native TS port. Native pg-delta / migra diff via edge-runtime against a Go-seam-provisioned live shadow (`db __shadow`); `--use-pgadmin` / `--use-pg-schema` delegate to the Go binary. `--use-pg-schema` is deprecated (CLI-1960: TS-only stderr warning + `--help` note) in favor of the pg-delta engine or the default migra engine — it wraps the in-process `stripe/pg-schema-diff` Go library, which has no TS/container equivalent, so it is a documented keep-in-Go exception, not a pending port. It will be the sole remaining Go delegation once `--use-pgadmin`, the `db __shadow`/`db __db-bootstrap` seams, and the other in-flight M9 issues are done. | | ||
| | `db diff` | `ported` | `legacy/commands/db/diff/` | `n/a` | `n/a` | Native TS port. Pg-delta runs in-process by default with bundled pg-topo against isolated Go-seam-provisioned shadows (`db __shadow`); `SUPABASE_USE_PG_DELTA_NEXT=false` retains the legacy edge-runtime implementation. Migra remains edge-runtime-backed; `--use-pgadmin` / `--use-pg-schema` delegate to the Go binary. `--use-pg-schema` is deprecated (CLI-1960: TS-only stderr warning + `--help` note) in favor of the pg-delta engine or the default migra engine — it wraps the in-process `stripe/pg-schema-diff` Go library, which has no TS/container equivalent, so it is a documented keep-in-Go exception, not a pending port. It will be the sole remaining Go delegation once `--use-pgadmin`, the `db __shadow` seam, and the other in-flight M9 issues are done. | |
There was a problem hiding this comment.
This change adds the TS-only --strict-coverage flag to db diff, but the updated tracker row still reports n/a under Extra TS flags/params. That leaves the command-surface parity record inaccurate and violates the workspace requirement to record added flags on already-ported commands; list --strict-coverage here (and audit the analogous changed leaves).
AGENTS.md reference: apps/cli/AGENTS.md:L453-L457
Useful? React with 👍 / 👎.
| for _, line := range m.Statements { | ||
| batch.ExecParams(line, nil, nil, nil, nil) | ||
| batchSize++ |
There was a problem hiding this comment.
When a migration contains an authored transaction followed by a standalone-only statement—for example BEGIN; …; COMMIT; CREATE INDEX CONCURRENTLY …—this branch queues every statement into pgconn.ExecBatch, bypassing the isPipelineIncompatible handling below. CREATE INDEX CONCURRENTLY, VACUUM, and the other recognized statements cannot execute through that pipeline, so otherwise valid migrations fail during Go application and while provisioning the default pg-delta migration shadow; preserve authored transaction boundaries while still flushing these statements through the standalone execution path.
Useful? React with 👍 / 👎.
| if transactional { | ||
| if _, err := conn.Exec(ctx, "BEGIN"); err != nil { | ||
| return errors.Errorf("failed to begin migration transaction: %w", err) | ||
| } |
There was a problem hiding this comment.
The new unconditional BEGIN around ordinary batches breaks internal/db/reset.recreateDatabase, which deliberately sends DROP DATABASE and CREATE DATABASE statements through MigrationFile.ExecBatch. PostgreSQL rejects those commands inside a transaction block, so the Go PG14 reset path now fails before recreating postgres; treat database-level commands as standalone or preserve the prior non-transactional execution contract for this internal batch.
Useful? React with 👍 / 👎.
| ), | ||
| ); | ||
| yield* legacyStartInitSchema(spawner, input, tmpDir); | ||
| yield* legacyApplyDatabaseWebhooks(input, tmpDir); |
There was a problem hiding this comment.
For a supported Postgres 14 project with [experimental.webhooks] enabled = true, fresh setup installs pg_net here, but the native TS PG14 reset path still runs only legacyInitSchema14 followed by legacyApplyApiPrivileges. Because this commit also removes pg_net from the PG14 initial-schema template, db reset drops the database and never restores the configured extension, unlike the updated Go initDatabase, which now calls ApplyDatabaseWebhooks; expose and reuse this step from the reset composition.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
1 participant
Runs pg-delta and pg-topo in-process for diff, pull, and declarative schema workflows, enabled by default behind a shared strategy boundary.
The existing edge-runtime implementation remains available through SUPABASE_USE_PG_DELTA_NEXT=false, with no automatic fallback. New-engine snapshots and diagnostics use an isolated v2 artifact format, and migration rendering preserves execution-required transaction boundaries.
Dependencies are temporarily pinned to pg-toolbelt PR #299 at commit 951daa9 and should move to released versions after publication. Generated SQL may differ from the legacy renderer; the compatibility contract is successful execution and state convergence.
The current shared live harness does not provision project Postgres, so linked-project, TLS-required, and pooler/SNI acceptance still requires a provisioned data-plane environment.